-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update closing HTML tags in form_helper.php #6658
Update closing HTML tags in form_helper.php #6658
Conversation
Dont forget changes unit tests for this PR |
Sorry, this is not a bug fix, so please send it to |
Please remove the space at the end. |
You forgot GPG signing. |
Im not sure that removing the closing tag is the best way to go here. The closing tag is needed for the more stricter XHTML. XHTML requires all elements to be closed including void elements. The closing tag on a plain HTML document is still valid even if some validation says its not. It is simply ignored. By omitting these you are causing problems for those who want to use XHTML markup. The presence of them has no affect on plain HTML markup. If you really have to get rid of them then I would have a config definition to decide how to render the tags. |
I think it is better to follow the standard. XHTML 1.1 specification is Superseded Recommendation. |
Do you need to support XHTML in year 2022? |
I don't use it with codeigniter but I do in Salesforce. Just saying some people might be using it and if you change it it could cause problems. |
I at least think this is a breaking change. |
No activity by author, i can send PR at home later |
One of criteria SEO is data structure schema must be Google said on https://developers.google.com/search/docs/appearance/structured-data
And now based on HTML5, we must support standard structure. |
@kenjis I have problem in my env with PHPStan 1.8.10 ------ ----------------------------------------------------------------------------------------------------------------------------
Line system\Helpers\form_helper.php
------ ----------------------------------------------------------------------------------------------------------------------------
Ignored error pattern #^Offset 'checked' on array\{type\: 'checkbox', name\: mixed, value\: string\} in isset\(\) does not
exist\.$# in path D:\Project\laragon\www\ci4form\system\Helpers\form_helper.php was not matched in reported errors.
366 Offset 'checked' on array{type: 'checkbox', name: string, value: string} in isset() does not exist.
------ ----------------------------------------------------------------------------------------------------------------------------
[ERROR] Found 2 errors
Fix the phpstan error(s) before commit. function form_checkbox($data = '', string $value = '', bool $checked = false, $extra = ''): string
{
$defaults = [
'type' => 'checkbox',
'name' => (! is_array($data) ? $data : ''),
'value' => $value,
];
if (is_array($data) && array_key_exists('checked', $data)) {
$checked = $data['checked'];
if ($checked === false) {
unset($data['checked']);
} else {
$data['checked'] = 'checked';
}
}
if ($checked === true) {
$defaults['checked'] = 'checked';
} elseif (isset($defaults['checked'])) {
unset($defaults['checked']);
}
return '<input ' . parse_form_attributes($data, $defaults) . stringify_attributes($extra) . ">\n";
} |
@ddevsr I cannot reproduce. |
Okay, i will send PR with PHPStan 1.8.8 |
See #6717 |
Description
Fixes #6649
Removed the '/' in closing tags for input fields for text, upload and checkbox.
Checklist: